Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add random forest prior to Bayesian Optimization #81

Merged
merged 4 commits into from
May 22, 2018
Merged

add random forest prior to Bayesian Optimization #81

merged 4 commits into from
May 22, 2018

Conversation

libbyandhelen
Copy link
Contributor

This PR add random forest as prior to Bayesian Optimization algorithm. RF is more suitable for discrete and categorical parameters, while gaussian process if more suitable for categorical parameters.

@libbyandhelen
Copy link
Contributor Author

/assign @gaocegege @YujiOshima

@YujiOshima
Copy link
Contributor

@libbyandhelen Thank you!
For validation of logic, can you add any citation for this algorithm?
If there is any difference to the citation, please specify the difference.

@libbyandhelen
Copy link
Contributor Author

@YujiOshima
The reference paper is

Frank Hutter, Holger H Hoos, and Kevin Leyton-Brown. 2011. Sequential modelbased
optimization for general algorithm configuration.
https://www.cs.ubc.ca/~hutter/papers/10-TR-SMAC.pdf

shall I put this into the code?

@gaocegege
Copy link
Member

I do not think we should put the paper citation in the code. Maybe we could maintain a doc about the algorithms that we support and list the citations there. WDYT

@YujiOshima
Copy link
Contributor

Yes, I agree with @gaocegege .
It should be put in docs.
You can add BO to README and cite here.

@libbyandhelen
Copy link
Contributor Author

@YujiOshima @gaocegege
The citation is added to the doc, which is for the whole BO algorithm in general, and it mentions the use of random forest prior.

@gaocegege
Copy link
Member

/ok-to-test

@libbyandhelen
Copy link
Contributor Author

/retest

@gaocegege
Copy link
Member

@libbyandhelen Please rebase the upstream/master and have a try 😄

@gaocegege
Copy link
Member

@YujiOshima PTAL

@YujiOshima
Copy link
Contributor

I'm going to take a look.

@YujiOshima
Copy link
Contributor

/lgtm

@YujiOshima
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: YujiOshima

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 04cf37f into kubeflow:master May 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ddysher ddysher Awaiting requested review from ddysher

@mitake mitake Awaiting requested review from mitake

Assignees

@gaocegege gaocegege

@YujiOshima YujiOshima

Labels
approved lgtm size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@libbyandhelen @YujiOshima @gaocegege @k8s-ci-robot